Skip to content

nixos/forgejo.runner: initialize module#496325

Closed
adamcstephens wants to merge 6 commits into
NixOS:masterfrom
adamcstephens:push-kpxuoykptuty
Closed

nixos/forgejo.runner: initialize module#496325
adamcstephens wants to merge 6 commits into
NixOS:masterfrom
adamcstephens:push-kpxuoykptuty

Conversation

@adamcstephens

@adamcstephens adamcstephens commented Mar 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I couldn't remember exactly where we left the discussion, and couldn't find the context looking at github/matrix, so I went ahead with an initial implementation. This is based off the gitea-actions-runner, but using a template service and applying maximum hardening. It's possible this is too much hardening and will need to be tuned back, but the basic tests at least are passing.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@tebriel

tebriel commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

this is very exciting, I'll attempt to replace my manual implementation with this tonight.

@nixpkgs-ci nixpkgs-ci Bot added 2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: documentation This PR adds or changes documentation labels Mar 3, 2026
tebriel
tebriel reviewed Mar 5, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
@tebriel

tebriel commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Ok I've moved my config to this module in https://frodux.dev/Frodux/nixos/pulls/772, it has properly registered, picked up a job, and is working on it. First test with docker as a runner working as expected.

mweinelt
mweinelt reviewed Mar 5, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix
@adamcstephens adamcstephens force-pushed the push-kpxuoykptuty branch 2 times, most recently from 615d282 to 41f14ec Compare March 5, 2026 21:09
@nixpkgs-ci nixpkgs-ci Bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 5, 2026
tebriel
tebriel reviewed Mar 5, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
@ofborg ofborg Bot added the ofborg-internal-error Ofborg encountered an error label Mar 5, 2026
@adamcstephens adamcstephens force-pushed the push-kpxuoykptuty branch 2 times, most recently from a0b1988 to dc3b027 Compare March 9, 2026 15:48
@adamcstephens

adamcstephens commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

This is looking pretty good to me here in nixpkgs. I still need to test this with my local exec runner to see if the hardening is ok.

@ofborg ofborg Bot removed the ofborg-internal-error Ofborg encountered an error label Mar 9, 2026
tebriel
tebriel reviewed Mar 10, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
tebriel
tebriel approved these changes Mar 11, 2026
View reviewed changes

@tebriel tebriel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still only running it in docker mode but the update to use LoadCredential is working as expected.

@adamcstephens
nixos/forgejo.runner: initialize module
ec3bc42 
Initial port from services.gitea-actions-runner, switching to systemd
template services and applying aggressive hardening.

Assisted-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adamcstephens
nixos/forgejo.runner: rename registration token and make optional
b890579
@adamcstephens
nixos/forgejo.runner: make top-level labels optional
c4a6813
@adamcstephens
nixos/forgejo.runner: add more legacy and pre-registered assertions
2b41024
@adamcstephens

adamcstephens commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

I ended up dropping the systemd escaping. It's problematic for a few of the options and I'm not convinced it's worth the problems and ugliness. Open to feedback if you disagree.

mweinelt
mweinelt reviewed May 25, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
wrench-exile-legacy
wrench-exile-legacy reviewed May 26, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
frantathefranta
frantathefranta reviewed May 27, 2026
View reviewed changes
Comment thread nixos/modules/services/continuous-integration/forgejo-runner.nix Outdated
@adamcstephens adamcstephens added the backport release-26.05 Backport PR automatically label May 29, 2026
@minz1

minz1 commented May 29, 2026

Copy link
Copy Markdown

If it's a large lift migrating off the static runner token, I'm willing to assist with this PR. ForgeJo is deprecating it, no? It seems strange to initialize this module using deprecated functionality

@adamcstephens

adamcstephens commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This PR supports both registration token (yes, deprecated) and the newer pre-registered settings.server.connections capability. I'd call the newer method "static runner token" while the former allows for dynamic registration.

Upstream does not have a timeline for removal of registration token, but it won't be while v15 LTS is supported.

@adamcstephens

adamcstephens commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Nobody willing to approve this? A number of people have weighed in and reviewed.

tebriel
tebriel approved these changes Jun 8, 2026
View reviewed changes

@tebriel tebriel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels 1.0-complete and ready to go. Users will need to opt-in to it so I'm not worried about breaking someone's existing configuration. Let's do this and fix any sharp edges we find along the way. I've been running this branch for quite some time so it's at least very ready based on my use cases.

wrench-exile-legacy
wrench-exile-legacy approved these changes Jun 8, 2026
View reviewed changes

@wrench-exile-legacy wrench-exile-legacy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 8, 2026
@mweinelt

mweinelt commented Jun 8, 2026

Copy link
Copy Markdown
Member

The reason I did not approve is that this feels too much stuck in the past. A fresh take would be fully config file driven.

@adamcstephens

adamcstephens commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

The reason I did not approve is that this feels too much stuck in the past. A fresh take would be fully config file driven.

Does this not also provide that capability?

@emilylange

emilylange commented Jun 8, 2026

Copy link
Copy Markdown
Member

If registration token is a blocker for you, let me know and I'll close this PR and let someone else pick it up.

I'll open a new PR with a different take on this in an hour or two.

@adamcstephens adamcstephens deleted the push-kpxuoykptuty branch June 8, 2026 15:12
@adamcstephens

adamcstephens commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Fair enough, I'll close this then and defer to your PR.

@emilylange emilylange mentioned this pull request Jun 8, 2026
Open
14 tasks
@emilylange

emilylange commented Jun 8, 2026

Copy link
Copy Markdown
Member

#529621 is ready for review now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mweinelt mweinelt mweinelt left review comments

@gepbird gepbird gepbird left review comments

@bendlas bendlas Awaiting requested review from bendlas

@christoph-heiss christoph-heiss Awaiting requested review from christoph-heiss

@pyrox0 pyrox0 Awaiting requested review from pyrox0

@NyCodeGHG NyCodeGHG Awaiting requested review from NyCodeGHG

@emilylange emilylange Awaiting requested review from emilylange

+5 more reviewers

@nrabulinski nrabulinski nrabulinski left review comments

@caniko caniko caniko left review comments

@frantathefranta frantathefranta frantathefranta left review comments

@tebriel tebriel tebriel approved these changes

@wrench-exile-legacy wrench-exile-legacy wrench-exile-legacy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. backport release-26.05 Backport PR automatically

Projects

None yet

Milestone

26.05

Development

Successfully merging this pull request may close these issues.